Skip to content

SNOW-1897976: adding pipe method to DataFrame#4178

Open
Tijoxa wants to merge 8 commits intosnowflakedb:mainfrom
Tijoxa:main
Open

SNOW-1897976: adding pipe method to DataFrame#4178
Tijoxa wants to merge 8 commits intosnowflakedb:mainfrom
Tijoxa:main

Conversation

@Tijoxa
Copy link
Copy Markdown

@Tijoxa Tijoxa commented Apr 15, 2026 •
edited
Loading

  1. Which Jira issue is this PR addressing? Make sure that there is an accompanying issue to your PR.

    Fixes SNOW-1897976

  2. Fill out the following pre-review checklist:

    • I am adding a new automated test(s) to verify correctness of my new code
      • If this test skips Local Testing mode, I'm requesting review from @snowflakedb/local-testing
    • I am adding new logging messages
    • I am adding a new telemetry message
    • I am adding new credentials
    • I am adding a new dependency
    • If this is a new feature/behavior, I'm adding the Local Testing parity changes.
    • I acknowledge that I have ensured my changes to be thread-safe. Follow the link for more information: Thread-safe Developer Guidelines
    • If adding any arguments to public Snowpark APIs or creating new public Snowpark APIs, I acknowledge that I have ensured my changes include AST support. Follow the link for more information: AST Support Guidelines

  3. Please describe how your code solves the related issue.

    This simple pipe method mimics the functionality of similar pipe methods in popular libraries like pandas or polars.

@Tijoxa Tijoxa requested review from a team as code owners April 15, 2026 15:54
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 15, 2026 •
edited
Loading

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@Tijoxa
Copy link
Copy Markdown
Author

Tijoxa commented Apr 15, 2026

I have read the CLA Document and I hereby sign the CLA

sfc-gh-joshi
sfc-gh-joshi requested changes Apr 15, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@sfc-gh-joshi sfc-gh-joshi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution. Please take a look (or have your agent take a look) at the comments I left.

Since this is a new API, please also add DataFrame.pipe to our API documentation reference in https://github.com/snowflakedb/snowpark-python/blob/main/docs/source/snowpark/dataframe.rst, and add a corresponding entry to CHANGELOG.md.

One question (mostly out of curiosity): is there a reason the github account you used to create the PR is different from the account that you used to create commits?

Comment thread src/snowflake/snowpark/dataframe.py Outdated
Comment thread src/snowflake/snowpark/dataframe.py Outdated
Comment thread tests/unit/test_dataframe.py Outdated
Comment thread tests/unit/test_dataframe.py Outdated
Comment on lines +415 to +421
df._plan._metadata = PlanMetadata(
attributes=[
Attribute("A", IntegerType(), False),
Attribute("B", StringType()),
],
quoted_identifiers=None,
)
Copy link
Copy Markdown
Contributor

@sfc-gh-joshi sfc-gh-joshi Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like you copied this example from test_dataFrame_printSchema in the same file, but we should avoid modifying internal dataframe attributes during tests whenever possible. I don't think this is necessary for this test regardless.

Also, please modify test_func to actually do something to the dataframe, since right now it's a no-op and not really a meaningful test.

Copy link
Copy Markdown
Author

@Tijoxa Tijoxa Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the new commits, I've updated the tests and tried to put the test at the correct place. However, I don't know if I should put it under tests/unit/test_dataframe.py, tests/integ/test_dataframe.py or any other directory.

@Tijoxa
Copy link
Copy Markdown
Author

Tijoxa commented Apr 16, 2026

Hi, thank you for your feedbacks.
The different accounts were just a mistake on my part. I didn't realize which one I was logged into on the computer!

@TimotheeMesnardCH
Copy link
Copy Markdown

TimotheeMesnardCH commented Apr 16, 2026

I have read the CLA Document and I hereby sign the CLA

@sfc-gh-joshi
Copy link
Copy Markdown
Contributor

sfc-gh-joshi commented Apr 17, 2026

It looks like there's some linting issues from CI: https://github.com/snowflakedb/snowpark-python/actions/runs/24531983812/job/71867812441?pr=4178

Please install pre-commit, which will automatically run isort/flake8/black on your dev machine when you make a commit. I can kick off the CI test suite for you once that's fixed.

@Tijoxa
Copy link
Copy Markdown
Author

Tijoxa commented Apr 17, 2026 •
edited
Loading

It looks like there's some linting issues from CI: https://github.com/snowflakedb/snowpark-python/actions/runs/24531983812/job/71867812441?pr=4178

Please install pre-commit, which will automatically run isort/flake8/black on your dev machine when you make a commit. I can kick off the CI test suite for you once that's fixed.

The linting issue pointed at the CI points to what my local formatter (Ruff) signals. However, it has not been fixed by the pre-commit that I've just installed. The pre-commit hook even flagged another line in the codebase that I haven't touched (it wants to use f-strings):

snowpark-python/src/snowflake/snowpark/modin/plugin/extensions/dataframe_overrides.py

Line 1304 in c5362e4

dtypes_line = f"dtypes: {', '.join(['{}({})'.format(dtype, count) for dtype, count in dtypes.value_counts().items()])}"

That may be due to the difficulties I've encountered for running pre-commit (I use uv on a Windows machine)...

Anyway I hope that the CI is happy now!

@sfc-gh-joshi
Copy link
Copy Markdown
Contributor

sfc-gh-joshi commented Apr 20, 2026

It seems like we had some recent changes to our internal auth mechanisms that prevent CI from running properly for external contributions. I've kicked off CI for this PR in #4189, which hopefully should pass.

@Tijoxa
Copy link
Copy Markdown
Author

Tijoxa commented Apr 22, 2026

It seems like we had some recent changes to our internal auth mechanisms that prevent CI from running properly for external contributions. I've kicked off CI for this PR in #4189, which hopefully should pass.

Do you mean that the merge is happening on your branch? Do you need me to do anything particular in #4189 , like signing the CLA Document?

@sfc-gh-joshi
Copy link
Copy Markdown
Contributor

sfc-gh-joshi commented Apr 22, 2026

I just need reviews from team members to approve the PR. It might take a bit to get merged, but we'll try to get it in before our next release cut.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sfc-gh-joshi sfc-gh-joshi sfc-gh-joshi requested changes

@sfc-gh-yuwang sfc-gh-yuwang Awaiting requested review from sfc-gh-yuwang sfc-gh-yuwang is a code owner automatically assigned from snowflakedb/snowpark-python-api-reviewers

@sfc-gh-aalam sfc-gh-aalam Awaiting requested review from sfc-gh-aalam sfc-gh-aalam is a code owner automatically assigned from snowflakedb/snowpark-python-api-reviewers

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

SNOW-1897976: Consider adding a new DataFrame method to allow applying transformations in the fluent-style

3 participants

@Tijoxa @TimotheeMesnardCH @sfc-gh-joshi